Normalizes CRLF line endings at certificate input boundary#787
Merged
hemanthnakkina merged 1 commit intocanonical:mainfrom Apr 25, 2026
Merged
Normalizes CRLF line endings at certificate input boundary#787hemanthnakkina merged 1 commit intocanonical:mainfrom
hemanthnakkina merged 1 commit intocanonical:mainfrom
Conversation
CRLF line endings are surviving validation, causing duplicate entries in ca_bundle.pem and inconsistent data stored in keystone. Fixes by normalizing CRLF to LF at two input boundaries: - validate_ca_certificate / validate_ca_chain: strips CRLF from --ca and --ca-chain - ConfigureTLSCertificatesStep.prompt() in common.py: strips CRLF from signed leaf certs 0bb84d3 partially fixed this but missed sanitizing the --ca which gets carried to keystone and causes duplication later in ca_bundle.pem. With normalization at the input boundary, this fix is no longer needed Signed-off-by: Raven Kaur <raven.kaur@canonical.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses inconsistent certificate storage caused by CRLF line endings by normalizing PEM content to LF at key certificate input boundaries, preventing downstream duplication (e.g., in ca_bundle.pem) and inconsistent Keystone data.
Changes:
- Added a shared
normalize_pem()helper and applied it invalidate_ca_certificate/validate_ca_chain, returning canonicalized base64. - Normalized signed leaf certificates in
ConfigureTLSCertificatesStep.prompt()before persisting answers. - Updated/added unit tests covering CRLF normalization behavior in the validators and prompt flow.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
sunbeam-python/sunbeam/features/interface/utils.py |
Introduces PEM normalization and canonicalizes CA/chain validator outputs; removes prior normalization from generate_ca_chain(). |
sunbeam-python/sunbeam/features/tls/common.py |
Normalizes leaf certs (base64 decode → PEM normalize → base64 encode) before storing in answers/state. |
sunbeam-python/tests/unit/sunbeam/features/test_utils.py |
Adds unit tests asserting CRLF normalization for CA certificate/chain validators. |
sunbeam-python/tests/unit/sunbeam/features/test_tls.py |
Adjusts fixtures to accommodate new base64/normalization behavior in the TLS prompt path. |
Comments suppressed due to low confidence (1)
sunbeam-python/sunbeam/features/interface/utils.py:225
generate_ca_chain()no longer normalizes CRLF/CR before checking whetherca_certificate_decodedis already present inca_chain_decoded. There are existing callers (e.g., load balancer certificate prompts) that don’t appear to normalize CA cert/chain inputs, so this can reintroduce duplicated CA certs when line endings differ. Consider either keeping normalization insidegenerate_ca_chain()(to make it robust for all callers) or normalizing CA cert/chain at every input boundary that can reach this function.
certificate_decoded = decode_base64_as_string(certificate)
ca_certificate_decoded = decode_base64_as_string(ca_certificate)
ca_chain_decoded = decode_base64_as_string(ca_chain)
if not certificate_decoded or not ca_certificate_decoded or not ca_chain_decoded:
raise binascii.Error("Unable to decode one of the certificates")
# If ca_certificate is already part of ca_chain, do not add it to the final ca chain
# manual-tls-certificates checks if the final ca_chain is in proper order and each
# certificate is signed by the successor one.
if ca_certificate_decoded in ca_chain_decoded:
chain_parts = [certificate_decoded, ca_chain_decoded]
else:
chain_parts = [certificate_decoded, ca_certificate_decoded, ca_chain_decoded]
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
hemanthnakkina
approved these changes
Apr 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CRLF line endings are surviving validation, causing duplicate entries in ca_bundle.pem and inconsistent data stored in keystone.
Fixes by normalizing CRLF to LF at two input boundaries:
0bb84d3 partially fixed this but missed sanitizing the --ca which gets carried to keystone and causes duplication later in ca_bundle.pem. With normalization at the input boundary, this fix is no longer needed